Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frame, member and project detail layout #67

Merged
merged 2 commits into from
Dec 3, 2024
Merged

Frame, member and project detail layout #67

merged 2 commits into from
Dec 3, 2024

Conversation

Soecka
Copy link
Contributor

@Soecka Soecka commented Oct 26, 2024

PR-67 PR-67 PR-67 Powered by Pull Request Badge

Co-authored-by: South Drifted [email protected]

Checklist(清单):

  • Labels
  • Assignees
  • Reviewers

[add] i18n switch, LarkImage component
[polish] project, member detail pages
@Soecka Soecka added the enhancement Some improvements label Oct 26, 2024
@Soecka Soecka added this to the 官网3.0 milestone Oct 26, 2024
@Soecka Soecka requested a review from TechQuery October 26, 2024 07:39
Comment on lines -22 to +27
<div className="container mx-auto max-w-screen-xl">
<PageHead title={t('member')} />

<h1 className="my-8">{t('member')}</h1>

<ScrollList
translator={i18n}
store={memberStore}
renderList={allItems => <MemberListLayout defaultData={allItems} />}
defaultData={list}
/>
</div>
<Frame
title={t('member')}
header={t('member')}
store={memberStore}
Layout={MemberListLayout}
defaultData={list}
/>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没必要这么高度封装吧?原有三者没有必然强关联,<Frame /> 的命名也应该只包含容器、标题,而非具体内容。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

三个都是 PageHeadh1ScrollList 的结构,我可以更改命名为 FrameListLayout

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

三个都是 PageHeadh1ScrollList 的结构,我可以更改命名为 FrameListLayout

如果非要这样的话,命名应该是 ScrollListPage

export const blobURLOf = (value: TableCellValue) =>
value instanceof Array
? typeof value[0] === 'object' && ('file_token' in value[0] || 'attachmentToken' in value[0])
? `${fileBaseURI}/${value[0].name}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

不要再用文件名了,用 token。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 那个 PR 合了调通了再改,blobClient 这些都要改

Comment on lines +46 to 49
{/**
* @todo replace with LarkImage after R2 is ready
*/}
<img className="object-fill" src={fileURLOf(project.image)} alt={String(project.name)} />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

现在可以直接用包装组件了,因为有自动的飞书后备接口。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 那个 PR 合了调通了再改,blobClient 这些都要改

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 那个 PR 合了调通了再改,blobClient 这些都要改

包装组件也是调用的同样的 fileURLOf(),逻辑和路径拼接是解耦的,相互不影响。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

R2 PR 调完一起改吧,现在改了合了 PR 还是要再提 PR 改

@Soecka Soecka requested a review from TechQuery November 29, 2024 13:31
@TechQuery TechQuery merged commit 570cbc7 into v3 Dec 3, 2024
1 check passed
@TechQuery TechQuery deleted the detail branch December 3, 2024 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Some improvements
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants